[gitpod-protocol] handle host:port:token for getGitpodImageAuth #20806
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR improves the parsing of the Gitpod image authentication environment variable by supporting both "host:token" and "host:port:token" formats. In addition to updating the splitting logic in getGitpodImageAuth, comprehensive unit tests have been added to cover various scenarios including malformed inputs and entries with extra spaces.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| components/gitpod-protocol/src/protocol.ts | Updated getGitpodImageAuth to correctly handle host:token and host:port:token formats. |
| components/gitpod-protocol/src/protocol.spec.ts | Added tests for the new parsing logic and ensured various cases (empty, malformed, multiple entries) are covered. |
There was a problem hiding this comment.
@kylos101 We have at least two other places where we parse this forward, which we need to adjust as well.
IIRC it was in proxy.go (image-builder-bob), and maybe even somewhere else - maybe around docker-up? 🤔
I can pull out the references on Monday - or you try to ask cline 😉
Thanks again for the tip, @geropl! It seems supervisor is impacted here,image-builder-bob here and here, and perhaps image-builder-mk3 here are impacted (I'll start with supervisor). |
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the Gitpod image authentication mechanism by supporting both "host:" and "host:port:" formats. Key changes include:
- Introducing a helper function getHost to determine the host (with optional port) from the token.
- Updating the image authentication parsing logic accordingly.
- Adding extensive unit tests to cover various input scenarios for image authentication.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| components/gitpod-protocol/src/protocol.ts | Added helper function and updated parsing logic for handling host with optional port |
| components/gitpod-protocol/src/protocol.spec.ts | Added new tests to verify correct handling of image auth entries, including edge cases |
|
Related to #20586 |
There was a problem hiding this comment.
Pull Request Overview
This PR improves the parsing and handling of the GITPOD_IMAGE_AUTH environment variable, now supporting both "host:token" and "host:port:token" formats. Key changes include an update to the credential parsing logic in supervisor/docker.go and its corresponding tests, enhancements to additional authentication handling in image-builder-mk3, and modifications to the Gitpod protocol parsing in protocol.ts along with new tests in protocol.spec.ts.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| components/supervisor/pkg/supervisor/docker_test.go | Added tests to verify the parsing of credentials, including host with port and various malformed cases. |
| components/supervisor/pkg/supervisor/docker.go | Updated the insertion logic to support splitting credentials into host, port (if provided), and token. |
| components/image-builder-mk3/pkg/auth/auth_test.go | New tests covering additional auth scenarios including host:port tokens and edge cases. |
| components/image-builder-mk3/pkg/auth/auth.go | Modified authentication extraction to support credentials with multiple colon separators. |
| components/gitpod-protocol/src/protocol.ts | Updated getGitpodImageAuth to utilize a helper that distinguishes between host and host:port scenarios. |
| components/gitpod-protocol/src/protocol.spec.ts | New test suite ensuring proper parsing across various credential formats, including handling of extra spaces. |
Special case is to strip port for 443
|
@kylos101 Left a bunch of comments with the intention to not to block or nitpick, but aim for leaving all these code pieces (that are already in a bad spot because of these cross-component dependencies) in a better spot than before. |
|
Merging after we did extensive testing: https://linear.app/gitpod/issue/CLC-1364/image-pull-does-not-work-with-hostporttoken#comment-cbc77632 |
|
/unhold |
Description
Be able to consume GITPOD_IMAGE_AUTH, where it is
host:<token>orhost:port:<token>.Related Issue(s)
Fixes #16237
Fixes CLC-1364
How to test
See unit tests
Test from a preview environment
Documentation
Preview status
gitpod:summary
Build Options
Build
Run the build with werft instead of GHA
Run Leeway with
--dont-testPublish
Installer
Add desired feature flags to the end of the line above, space separated
Preview Environment / Integration Tests
If enabled this will build
install/previewIf enabled this will create the environment on GCE infra
Saves cost. Untick this only if you're really sure you need a non-preemtible machine.
Valid options are
all,workspace,webapp,ide,jetbrains,vscode,ssh. If enabled,with-previewandwith-large-vmwill be enabled./hold